Scorecard Integration #1294
Scorecard Integration #1294
Conversation
developed functions to check for availability nexB#598 Signed-off-by: 404-geek <[email protected]>
Signed-off-by: 404-geek <[email protected]>
Signed-off-by: 404-geek <[email protected]>
Signed-off-by: 404-geek <[email protected]>
Signed-off-by: 404-geek <[email protected]>
…aving logic nexB#1283 Signed-off-by: 404-geek <[email protected]>
Signed-off-by: 404-geek <[email protected]>
Signed-off-by: 404-geek <[email protected]>
…exB#1283 Signed-off-by: 404-geek <[email protected]>
Signed-off-by: 404-geek <[email protected]>
Signed-off-by: 404-geek <[email protected]>
Signed-off-by: 404-geek <[email protected]>
Signed-off-by: 404-geek <[email protected]>
… nexB#598 Signed-off-by: 404-geek <[email protected]>
…ecard_integration
…up.cfg nexB#1283 Signed-off-by: 404-geek <[email protected]>
Signed-off-by: 404-geek <[email protected]>
…gration # Conflicts: # scanpipe/models.py # scanpipe/tests/test_models.py
Signed-off-by: 404-geek <[email protected]>
Signed-off-by: 404-geek <[email protected]>
Signed-off-by: 404-geek <[email protected]>
Signed-off-by: 404-geek <[email protected]>
Signed-off-by: 404-geek <[email protected]>
Signed-off-by: 404-geek <[email protected]>
|
@404-geek can you check the failing tests? |
|
Hey @404-geek , what's your latest status on this PR? Any chances we can complete and merge it before it diverges too much from the main branch? |
Hi @tdruez, I’m planning to have this PR ready by next week. Most of the requested changes have been addressed; I just need to add a few test cases before pushing the final version. |
Signed-off-by: 404-geek <[email protected]>
…ecard_integration
Signed-off-by: 404-geek <[email protected]>
Signed-off-by: 404-geek <[email protected]>
Signed-off-by: 404-geek <[email protected]>
Signed-off-by: 404-geek <[email protected]>
Signed-off-by: 404-geek <[email protected]>
Signed-off-by: 404-geek <[email protected]>
Signed-off-by: 404-geek <[email protected]>
Signed-off-by: 404-geek <[email protected]>
Signed-off-by: 404-geek <[email protected]>
scanpipe/migrations/0070_alter_project_purl_discoveredpackagescore_and_more.py
Outdated
Show resolved
Hide resolved
scanpipe/models.py
Outdated
| ) | ||
|
|
||
| @classmethod | ||
| @transaction.atomic() |
There was a problem hiding this comment.
@404-geek You haven't address the question above yet ;)
Signed-off-by: 404-geek <[email protected]>
Signed-off-by: 404-geek <[email protected]>
…ecard_integration
Signed-off-by: 404-geek <[email protected]>
Signed-off-by: 404-geek <[email protected]>
| @@ -128,6 +130,7 @@ dev = | |||
| android_analysis = | |||
| android_inspector==0.0.1 | |||
|
|
|||
|
|
|||
| package1.discovered_packages_score.filter(scoring_tool="ossf-scorecard")[ | ||
| 0 | ||
| ].score, |
There was a problem hiding this comment.
Put package1.discovered_packages_score.filter(scoring_tool="ossf-scorecard")[0] in a variable for better readability.
There was a problem hiding this comment.
Also, could we test on the real value in place of assertIsNotNone?
| msg=out, | ||
| ) | ||
|
|
||
| self.assertEqual("https://github.com/ossf/scorecard", package1.vcs_url) |
There was a problem hiding this comment.
Why test this value here since it was set earlier in the test? Is it expected to change?
| @@ -1238,6 +1238,38 @@ def test_scanpipe_find_vulnerabilities_pipeline_integration( | |||
| expected = vulnerability_data[0]["affected_by_vulnerabilities"] | |||
| self.assertEqual(expected, package1.affected_by_vulnerabilities) | |||
|
|
|||
| @mock.patch("scorecode.ossf_scorecard.is_available") | |||
| def test_scanpipe_get_scorecard_info_packages_integration(self, mock_is_available): | |||
There was a problem hiding this comment.
This test depends on internet access at the moment. This is problematic.
The ossf_scorecard.fetch_scorecard_info return value should be mocked instead.
| scorecard_data_file.parent.mkdir(parents=True, exist_ok=True) | ||
|
|
||
| scorecard_data_file.write_text(json.dumps(scorecard_data, indent=2)) | ||
|
|
||
| print(f"Scorecard data successfully saved to {scorecard_data_file}") |
There was a problem hiding this comment.
This should be outside the try block.
| return None | ||
|
|
||
| @classmethod | ||
| @transaction.atomic() |
| @classmethod | ||
| def create_from_package_and_scorecard(cls, scorecard_data, package): | ||
| score_object = cls.create_from_scorecard_data( | ||
| discovered_package=package, | ||
| scorecard_data=scorecard_data, | ||
| scoring_tool="ossf-scorecard", | ||
| ) | ||
| return score_object |
There was a problem hiding this comment.
Is this one really useful?
It seems to create complexity around create_from_scorecard_data with simply defining a scoring_tool value.
I would suggested to get rid of it and define a default value for the scoring_tool:
def create_from_scorecard_data(
cls, discovered_package, scorecard_data, scoring_tool="ossf-scorecard"
):
There was a problem hiding this comment.
Also, why the scoring_tool is not part of the scorecard_data?
It seems weird to allow to overwrite the scoring_tool value from the parameter but still taking the tool_version and docuementation_url from the scorecard_data.
| package=package, scorecard_data=scorecard_obj | ||
| ) | ||
|
|
||
| self.assertIsNotNone(package_score) |
| if check.check_score == "-1": | ||
| self.assertEqual(check.check_score, "-1") |
| package=package, scorecard_data=scorecard_obj | ||
| ) | ||
|
|
||
| # Step 4: Retrieve the first check that was automatically created |
ScoreCode Integration
This pull request integrates the ScoreCode Repo into SCIO, enabling the fetching of the latest OSSF Scorecard Data for
discovered packagesusing theirvcs_url. The current implementation supportsgithub.comandgitlab.comVCS URLs.Key Features:
vcs_urlgithub.comandgitlab.comVCS URLsRelated Issues:
This feature enhances SCIO's functionality by ensuring that users can retrieve the most up-to-date security scores for packages discovered in their projects, improving overall security assessment and management.